New-style tasks/workflows use user exception scope #540
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
New-style tasks/workflows are currently not wrapped in a user exception scope, so all exceptions in user code are treated as unknown system errors which are automatically retried. Further context - https://flyte-org.slack.com/archives/CREL4QVAQ/p1625581856326400 This PR restores the user exception scope to new-style tasks/workflows so exceptions in user code are not retried unless FlyteRecoverableException is raised.
slai
commented
Jul 7, 2021
| @@ -96,7 +96,11 @@ def _dispatch_execute( | |||
| c: OR if an unhandled exception is retrieved - record it as an errors.pb | |||
| """ | |||
| output_file_dict = {} | |||
| try: | |||
|
|
|||
| @_scopes.system_entry_point | |||
There was a problem hiding this comment.
Although _dispatch_execute is already being executed in a system exception scope, that scope is too far up the stack to wrap exceptions raised in this function so the subsequent except blocks below can handle them.
Codecov Report
@@ Coverage Diff @@
## master #540 +/- ##
=======================================
Coverage 85.37% 85.37%
=======================================
Files 371 371
Lines 28710 28724 +14
Branches 2310 2310
=======================================
+ Hits 24510 24523 +13
- Misses 3561 3562 +1
Partials 639 639
Continue to review full report at Codecov.
|
|
Closing as #543 has fixed this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
New-style tasks/workflows are currently not wrapped in a user exception scope, so all exceptions in user code are treated as unknown system errors which are automatically retried. Further context - https://flyte-org.slack.com/archives/CREL4QVAQ/p1625581856326400
This PR restores the user exception scope to new-style tasks/workflows so exceptions in user code are not retried unless FlyteRecoverableException is raised.
Type
Are all requirements met?
Complete description
Based on https://github.com/flyteorg/flytekit/pull/440/files, but I tried to keep the user scope as tightly restricted to user code as possible.